-
Notifications
You must be signed in to change notification settings - Fork 798
Support for boolean data types as metrics #202
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support for boolean data types as metrics #202
Conversation
This looks like it has linter errors. |
Ok, will work on it right now. |
@wrouesnel checks passed, ready to merge whenever you are ready. |
It seems that the functions dbToFloat64 and dbToString are not covered by tests. Since this is the case, how would I increase coverage from coveralls, given that the changes I made are in those functions? |
It doesn't look like the CI error was introduced by my change. @wrouesnel
|
@miguelHx Generally that happens when the smoke-tests fail in some way. But I'll take a look!
(I should look into getting the build to put metalinter alerts on stderr or in a better color) |
@wrouesnel I ran the command |
@wrouesnel can I get some eyes on this? I see some parsing errors in the build log but I don't see how that relates to my code updates. I think this is what's preventing a successful build (near the top of the build log):
I can't find that particular script. Any help would be appreciated. |
Found the script, going to take a look. |
@wrouesnel I'm looking at the script: #!/bin/bash
# Script to setup the assets clone of the repository using GIT_ASSETS_BRANCH and
# GIT_API_KEY.
[ ! -z "$GIT_ASSETS_BRANCH" ] || exit 1
[ ! -z "$GIT_API_KEY" ] || exit 1
setup_git() {
git config --global user.email "travis@travis-ci.org" || exit 1
git config --global user.name "Travis CI" || exit 1
}
# Constants
ASSETS_DIR=".assets-branch"
# Clone the assets branch with the correct credentials
git clone --single-branch -b "$GIT_ASSETS_BRANCH" \
"https://${GIT_API_KEY}@github.com/${TRAVIS_REPO_SLUG}.git" "$ASSETS_DIR" || exit 1 Do I need to update something in my branch to make this work? I'm a little lost lol. |
Added some print statements in the script so narrow down where the script is exiting. |
As per my issue #235, now the build passes after the small |
triggering an external build to test out latest change |
@wrouesnel I think this PR is ready to merge now :) |
There are many commits in this PR, so if you could click the option to merge all of this as one commit, that would be great. |
@wrouesnel Will this be merged any time soon? Are there any other issues with this PR?:) |
6319433
to
a477f26
Compare
Ah right, it needs a rebase. I was meaning to click the merge button a while back. I've pushed a best guess at the rebase back to the PR, we'll see if it builds properly... |
@wrouesnel Okay, looks like it built properly. Let me know if any concerns arise. |
@wrouesnel Is the rebase complete? |
@wrouesnel my apologies, I realize that I should do the rebase. I haven't done a rebase before, so I'll do some research and I'll make it happen. |
11748c4
to
9b3feaa
Compare
9b3feaa
to
cc168f3
Compare
squashed my commits and rebased onto the upstream master. Build failed, I'm worried that my rebase may have had something to do with it :0, but could be a different issue |
04b7d65
to
cc168f3
Compare
This is useful if your database uses true/false for state and want to make prometheus alerts based on that. Before, booleans were not able to be parsed. See issue prometheus-community#201
d64651a
to
a3cd819
Compare
Rebased. I think it's ready to be merged :) |
Friendly reminder :) @wrouesnel |
This is useful if your database uses true/false for state and want to make prometheus alerts based on that.
Before, booleans were not able to be parsed. See issue #201